ENH: Added DataFrame.nsorted to select top n rows according to column-dependent order
#61457
Conversation
MartinBraquet
changed the title
DataFrame.nsorted to select top n rows according to column-dependent orderDataFrame.nsorted to select top n rows according to column-dependent order
MartinBraquet
changed the title
DataFrame.nsorted to select top n rows according to column-dependent orderDataFrame.nsorted to select top n rows according to column-dependent order
|
This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this. |
|
The PR has been waiting for reviewers since then. |
pandas/_typing.py
Outdated
| # Arguments for nsorted, nsmallest and nlargest. | ||
| NsmallestNlargestKeep = Literal["first", "last", "all"] |
There was a problem hiding this comment.
Why remove TypeAlias? Also, can you keep the style here consistent by not ending with a period .
There was a problem hiding this comment.
No reason, good catch! Note to myself: add the type after extracting a constant in Pycharm, because the IDE won't do it for me (so far).
pandas/core/frame.py
Outdated
| This method is equivalent to | ||
| ``df.nsorted(n, columns, ascending=False)``. |
There was a problem hiding this comment.
I'd prefer if this line was removed; including nsorted in the See Also section below seems sufficient.
pandas/core/frame.py
Outdated
| This method is equivalent to | ||
| ``df.nsorted(n, columns, ascending=True)``. |
pandas/core/methods/selectn.py
Outdated
| n = self.n | ||
| dtype = self.obj.dtype | ||
| if not self.is_valid_dtype_n_method(dtype): | ||
| raise TypeError(f"Cannot use method '{method}' with dtype {dtype}") | ||
| raise TypeError(f"Cannot use n-sorting with dtype {dtype}") |
There was a problem hiding this comment.
I think this is okay but wonder if Cannot sort dtype {dtype} might be more clear?
There was a problem hiding this comment.
I tend to agree with your point. I updated the message.
| Series.nlargest | ||
| Series.nsmallest | ||
| Series.nsorted |
There was a problem hiding this comment.
I added nsorted and moved the sorting methods to the "Reshaping, sorting" section
| """ | ||
| Return the top `n` elements, sorted in ascending or descending order. | ||
|
|
||
| Parameters | ||
| ---------- | ||
| n : int, default 5 | ||
| Return this many descending sorted values. | ||
| ascending : bool | ||
| If True, return the smallest `n` elements. If False, return | ||
| largest `n` elements. | ||
| keep : {'first', 'last', 'all'}, default 'first' | ||
| When there are duplicate values that cannot all fit in a | ||
| Series of `n` elements: | ||
|
|
||
| - ``first`` : return the first `n` occurrences in order | ||
| of appearance. | ||
| - ``last`` : return the last `n` occurrences in reverse | ||
| order of appearance. | ||
| - ``all`` : keep all occurrences. This can result in a Series of | ||
| size larger than `n`. | ||
|
|
||
| Returns | ||
| ------- | ||
| Series | ||
| The `n` top values in the Series. | ||
|
|
||
| See Also |
There was a problem hiding this comment.
Since nsorted got added to the series API docs, I believe it requires its proper docstring as well. There is a lot of redundancy with nlargest and nsmallest. Let me know if it should be done differently.
|
I applied the comments and the checks passed, so the PR should be ready for re-review. |
Co-authored-by: Irv Lustig <[email protected]>
doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.PR is ready for review.